Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Zod chain for union types #312

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

spencermaxfield
Copy link

The various chained validations were not being applied to the zod schemas generated by the use of oneOf, allOf, and anyOf in an OpenAPI spec. For example, this spec:

schema:
  type: object
  properties:
    union:
      oneOf:
        - type: string
          minLength: 1
        - type: string
          enum: ["value1", "value2"]

Woud generate this:

z.object({ union: z.union([z.string(), z.enum(["value1", "value2"])]) })

Where the expected output is this (which includes min(1) for the first string type:

z.object({ union: z.union([z.string().min(1), z.enum(["value1", "value2"])]) })

This PR addresses this by making using of getZodChain to append the chained validators to the sub-types within the composed types.

Copy link

vercel bot commented Nov 13, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
openapi-zod-client-rim4 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 13, 2024 8:14am

return code.assign(type.toString());
const chain = getZodChain({
schema: type.schema as SchemaObject,
meta: { ...meta, isRequired: true },
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm admittendly not entirely sure why isRequired: true is needed here and in the other places I added getZodChain calls. I tried excluding it at first, but it resulted in many test failures, and including it gave the results I was expecting.

const schemaType = type.schema.type.toLowerCase() as NonNullable<typeof schema.type>;
isObject = !isPrimitiveType(schemaType);
}
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isObject was unused, so this if/else to set its value was unneeded.

@@ -104,7 +104,7 @@ describe("export-all-types", () => {
expect(data).toEqual({
schemas: {
Settings: "z.object({ theme_color: z.string(), features: Features.min(1) }).partial().passthrough()",
Author: "z.object({ name: z.union([z.string(), z.number()]).nullable(), title: Title.min(1).max(30), id: Id, mail: z.string(), settings: Settings }).partial().passthrough()",
Author: "z.object({ name: z.union([z.string().nullable(), z.number()]).nullable(), title: Title.min(1).max(30), id: Id, mail: z.string(), settings: Settings }).partial().passthrough()",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up on line 69 you'll see Author's name property is set to nullable: true, so before this test was asserting union types did not have the chained validation applied to their sub-types

@@ -66,7 +66,7 @@ test("missing-zod-chains", async () => {
.passthrough();
const nulltype = z.object({}).partial().passthrough();
const anyOfType = z.union([
z.object({}).partial().passthrough(),
z.object({}).partial().passthrough().nullable(),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. On line 21 of this file, the anyOf type has nullable: true, so this test was asserting a lack of the support being added in this PR.

@spencermaxfield spencermaxfield marked this pull request as ready for review November 13, 2024 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant